-
Notifications
You must be signed in to change notification settings - Fork 255
[chore] Add event's to the namespace registry #2382 #2464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[chore] Add event's to the namespace registry #2382 #2464
Conversation
06aaa4a
to
d1c802d
Compare
dc0d7d5
to
9414d75
Compare
Signed-off-by: James Thompson <[email protected]>
9414d75
to
e21445d
Compare
A few thoughts on this:
|
Thanks for the feedback @jsuereth
Reason for 1 page per signal definition is so that we could have a namespace index page which would hopefully list everything in that namespace be it events, log records, metrics, spans etc. If we have 1 page it quickly grows out of hand and this approach is inline with the proposal to update the metric layout.
Yes can do
If possible I would get rid of the starts with altogether and just do filtering on the last mapping and is what is required for the general namespace page. |
@jsuereth Following on from the semconv meeting, I will do the following:
|
8e344c3
to
fe145fd
Compare
fe145fd
to
996f804
Compare
da3f7a1
to
8616dc2
Compare
8616dc2
to
1240a6d
Compare
42a3207
to
f517fd3
Compare
f517fd3
to
af5ee03
Compare
|
||
## Gen AI Assistant Message | ||
|
||
**Status:**  |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this (and all other gen_ai) event is not really usable without the rest of the GenAI convention it follows at least today
To make the usable:
- we need to express event requirement level in the yaml.
- find a way to link from here to the actual convention which consist of a group of events, spans and metrics.
- Find ways to bring these notes https://github.com/thompson-tomo/semantic-conventions/blob/42a320708d1a5bbdaad4cb955765b8f9ef926d24/docs/gen-ai/gen-ai-events.md?plain=1#L20-L39 - without them someone who discovers this even without the convention might not know how to migrate.
When it comes to GenAI events, I don't see how event registry is helpful. It breaks coherent document in multiple pieces without solving any problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to express event requirement level in the yaml.
There is already a weaver issue for this open-telemetry/weaver#406
find a way to link from here to the actual convention which consist of a group of events, spans and metrics.
When #2518 is merged in the template used can add a link to the namespace page (currently commented out). Hence solving this. Also Grouping ties into open-telemetry/weaver#406
Find ways to bring these notes https://github.com/thompson-tomo/semantic-conventions/blob/42a320708d1a5bbdaad4cb955765b8f9ef926d24/docs/gen-ai/gen-ai-events.md?plain=1#L20-L39 - without them someone who discovers this even without the convention might not know how to migrate.
Agree and was already captured as open-telemetry/weaver#802
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we talk about the benefits we get from event registry in MD and moving event definitions away from the corresponding conventions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
benefits we get from event registry in MD
I no longer see it as an event registry, originally I did when first raising PR. It is now adding events to the namespace registry alongside the other signals. The benefits would be things like readability, discoverability & consistency.
moving event definitions away from the corresponding conventions
This I don't get, when #2518 is merged you will have a single gen-ai page which is automatically generated which contains tables describing all of the events & entities which are defined. This will grow to include metrics, spans & attributes once their definitions can be fully defined in yaml and tooling can handle it.
What I do agree with is the way in which implementations are defined, can and should be improved. I will see if I can come with an interim solution for gen-ai.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notion of namespace is not clear and I'd recommend prototyping and socializing the end result before working on any new proposals. I don't believe it's productive to work on it without getting feedback and buy-in from @open-telemetry/specs-semconv-approvers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was actioning what was discussed in the last semconv meeting where it was socialized the end result of having a page showcasing the events available and being able to select one to see more details. It was explicitly mentioned that this index page would grow to include the other signals.
cf97d66
to
13ed1b2
Compare
13ed1b2
to
a9453cc
Compare
82840f4
to
d2d81ec
Compare
d2d81ec
to
6b5fd79
Compare
Progresses #2382
Changes
This introduces events to the registry so that they can reside alongside the entities registry. This provides pure code generated documentation for events which reduces maintenance effort and makes things more discoverable.
Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist
[chore]